-
Notifications
You must be signed in to change notification settings - Fork 4.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
API Fetch: Improve isMediaUploadRequest check #34417
Conversation
This worked for me as advertised, and when applied to #34389 it correctly paged the result set for galleries over 100 when per_page set to -1. I don't think I have enough background around this part of the code though to sign off on it. @youknowriad, can you see any gotchas with this since you initially added this middleware? The background is that this change is needed to switch the gallery |
97a4f68
to
cc7e3df
Compare
Size Change: +20 B (0%) Total Size: 1.04 MB
ℹ️ View Unchanged
|
Sorry for another ping, @youknowriad. I would love to hear your opinion about this change. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it was @azaozz that implemented this initially.
It does make sense to limit it to "post" requests (maybe "put" too if that's supported)?
Also, we do have an HTTP1 middleware that transforms "post" to "get" requests, we need to make sure it's applied after this one.
Thanks for the review, @youknowriad.
I think API currently only supports uploads via the
The |
Description
Updates
isMediaUploadRequest
check and to allowper_page=-1
get requests for media items.Fixes #34410.
How has this been tested?
Unit tests are running locally:
Running the following code in the browser console doesn't result in the error -
400: Invalid parameter(s): per_page
Check that
mediaUploadMiddleware
is applied to upload requests. This can be done by checking the "Request call stack" in the Network tab.Screenshots
Types of changes
Checklist:
*.native.js
files for terms that need renaming or removal).